[Unpackaged] Fix taskbar glomming due to AUMID#20064
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
lhecker
left a comment
lhecker
left a comment
There was a problem hiding this comment.
Neat code! One minor bug only.
| // GH#20053: The shell resolves taskbar grouping identity as: per-window AUMID > | ||
| // per-process AUMID > auto-derived from exe path. Before we started setting a | ||
| // process AUMID, both the pinned .lnk and the process used auto-derived | ||
| // identity, so they matched. Now that we set an explicit AUMID, a pinned .lnk | ||
| // that predates the AUMID change has no AUMID and still uses auto-derived | ||
| // identity, causing a mismatch and a duplicate taskbar button. |
There was a problem hiding this comment.
So LNK files essentially... cache? the AUMID that an app sets? Is that right?
So if my app ever changes the ID all LNKs are invalid?
There was a problem hiding this comment.
Yup! The LNK can either have an AUMID explicitly set or it's derived by the OS. I've definitely had a few apps update and not glom to the same taskbar entry and I'm guessing it's because they set a new AUMID.
| // AUMID for THIS launch (both sides use auto-derived identity, so they match) | ||
| // and defer stamping the shortcut to process exit. On the next launch, the pin | ||
| // has our AUMID, so we set the process AUMID to match, and both agree. |
There was a problem hiding this comment.
Why not update the LNK immediately?
There was a problem hiding this comment.
See
terminal/src/cascadia/WindowsTerminal/WindowEmperor.cpp
Lines 405 to 409 in 73af681
If we stamp now, we get two choices:
- set process AUMID this launch: process stays auto-derived (old AUMID) + pin's identity switches to new AUMID = mismatch
- set process AUMID: while the window is being created, the shell is auto-deriving the AUMID with the LNK. Since the two operations aren't atomic, we get a mismatch.
Deferring means that we always have a match:
- first launch:
- LNK AUMID: auto-derived
- Process AUMID: auto-derived
- at shutdown:
- LNK AUMID: stamped with new AUMID
- Process AUMID: N/A (we've shut down)
- next launch:
- LNK AUMID: new AUMID
- Process AUMID: new AUMID
These were battle scars I earned while testing/implementing this change haha.
There was a problem hiding this comment.
I like this explanation more than the one in the code to be honest. 🙈 May be worth adjusting the comment IMO.
microsoft-github-policy-service
Bot
added
the
Needs-Author-Feedback
microsoft-github-policy-service
Bot
added
the
No-Recent-Activity
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
carlos-zamora
removed
Needs-Author-Feedback
This comment has been minimized.
This comment has been minimized.
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
so i have a potential third case for you
well, fourth
- link does not exist; do nothing (set our own autogenerated AUMID)
- link exists, same aumid (no work required, set it on our active process)
- link exists, no aumid (defer write of our aumid to the link)
- link exists, customized/different/unexpected AUMID (#8216)
also, it is possible to determine if you were launched from a .lnk and if so which specific one; this may be broadly safer than scanning only the taskbar pins, since it will let you read the aumid from any lnk file anywhere that launches Terminal (though: it is important to note that this will also only work for links to WindowsTerminal.exe and will not work for links to wt.exe unless we do specific work to propagate that through the startup stack)
microsoft-github-policy-service
Bot
added
the
Needs-Author-Feedback
The bug was caused by an AUMID mismatch between the Taskbar's .lnk file and Windows Terminal. Since no AUMID was associated with the .exe, the OS automatically creates one for us. However, microsoft#20018 added an AUMID for unpackaged scenarios, so now there was a mismatch, resulting in a new taskbar entry being created. To fix this, we check if a .lnk points to our .exe in the taskbar. There's 3 cases here: 1. no .lnk of interest exists --> set the AUMID normally 2. the .lnk carries our AUMID --> set the AUMID normally 3. the .lnk doesn't have an AUMID (aka uses the auto-resolved one) --> - for this launch: don't set the AUMID so that we both use the auto-resolved one - on next launch: set the AUMID on the .lnk and process so that they all agree ## Validation Steps Performed In unpackaged folder, move WindowsTerminal.exe to the taskbar (creates .lnk)... ✅ Double-click the .exe --> Same taskbar entry is used ✅ Double-click the .exe _again_ --> second window goes to same taskbar entry The first window doesn't have to close for this to work. It just * works *! Bug introduced in microsoft#20018 Closes microsoft#20053
4 participants
Summary of the Pull Request
The bug was caused by an AUMID mismatch between the Taskbar's .lnk file and Windows Terminal. Since no AUMID was associated with the .exe, the OS automatically creates one for us. However, #20018 added an AUMID for unpackaged scenarios, so now there was a mismatch, resulting in a new taskbar entry being created.
To fix this, we check if a .lnk points to our .exe in the taskbar. There's 3 cases here:
Validation Steps Performed
In unpackaged folder, move WindowsTerminal.exe to the taskbar (creates .lnk)...
✅ Double-click the .exe --> Same taskbar entry is used
✅ Double-click the .exe again --> second window goes to same taskbar entry
The first window doesn't have to close for this to work. It just * works *!
Bug introduced in #20018
Closes #20053